-
Notifications
You must be signed in to change notification settings - Fork 338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SASL EXTERNAL authentication mechanism #93
Conversation
@@ -44,15 +59,20 @@ class Login | |||
/** | |||
* Default constructor | |||
*/ | |||
Login() : _user("guest"), _password("guest") {} | |||
Login() : _mechanism(LOGIN_PLAIN), _user("guest"), _password("guest") {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use delegating constructor instead of copy-pasting
…ve LoginMechanisms definition into Login class.
Code refactored. |
What is the status of this Pull Request? This is something I was going to add, but found there was already a request for it. |
The status is that I dislike the code, but I have not yet had time to dig into this. I dont know how this sasl is supposed to work. But I suspect that we need some sort of base class that is extended by a plain login system and by a sasl login system. |
@EmielBruijntjes this PR work with certificate authentication |
Hi, this PR is also needed along with 224 for certificate auth to work. There is a bit missing still though - method to easily define when we use EXTERNAL mechanism. This is currently hidden in AMQP::Address. My current address.h test code assumes external auth at the moment when there is no username/pw in the URL. I'm sure there is nicer way to do that, but that would need bit more refactoring. |
I still don't like this code. The Login class is here to contain a username + password. If you don't need this, it makes more sense to not even construct it, doesn't it? |
Neither do I. but we do need to give the login type PLAIN vs EXTERNAL when establsihing session. That is ugliness in the protocol itself. Its a stupid thing that makes no sense - but we still have it! |
Are there currently ongoing developments on this topic? I'm willing to submit a PR introducing a Edit: I was looking for other mechanisms, it seems PLAIN and EXTERNAL are the only relevant: https://www.rabbitmq.com/access-control.html#mechanisms |
That makes sense, I will check it when you send in that request |
Added support for external (e.g. X509 client cert) authentication mechanism.
Code is backward-compatible (old usages of Login will keep working).
To authenticate using SASL EXTERNAL mechanism, pass following object as Login to Connection constructor:
AMQP::Login(AMQP::LOGIN_EXTERNAL)